Skip to content

ENH: wrap iinfo/finfo #294

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 8, 2025
Merged

ENH: wrap iinfo/finfo #294

merged 3 commits into from
Apr 8, 2025

Conversation

crusaderky
Copy link
Contributor

@Copilot Copilot AI review requested due to automatic review settings April 2, 2025 16:07
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for array arguments in the finfo and iinfo functions across multiple backend modules while ensuring compatibility with array API tests. Key changes include wrapping finfo and iinfo calls using the get_xp function in torch, numpy, cupy, and dask modules, and defining common implementations in the shared module.

Reviewed Changes

Copilot reviewed 5 out of 12 changed files in this pull request and generated no comments.

Show a summary per file
File Description
array_api_compat/torch/_aliases.py Added finfo and iinfo wrappers and updated all for torch backend
array_api_compat/numpy/_aliases.py Added finfo and iinfo wrappers for numpy backend
array_api_compat/dask/array/_aliases.py Introduced finfo and iinfo wrappers; all no longer exports them
array_api_compat/cupy/_aliases.py Added finfo and iinfo wrappers for cupy backend
array_api_compat/common/_aliases.py Implemented common finfo and iinfo definitions and updated all accordingly
Files not reviewed (7)
  • cupy-xfails.txt: Language not supported
  • dask-xfails.txt: Language not supported
  • numpy-1-21-xfails.txt: Language not supported
  • numpy-1-26-xfails.txt: Language not supported
  • numpy-dev-xfails.txt: Language not supported
  • numpy-xfails.txt: Language not supported
  • torch-xfails.txt: Language not supported
Comments suppressed due to low confidence (1)

array_api_compat/dask/array/_aliases.py:346

  • The public API (all) in the dask module no longer exports 'finfo' and 'iinfo', which is inconsistent with other backends. Consider adding 'finfo' and 'iinfo' back to all to maintain a consistent interface.
'bitwise_right_shift', 'concat', 'pow', 'can_cast',

cupy-xfails.txt Outdated
array_api_tests/test_data_type_functions.py::test_finfo[float32]
# attributes are np.float32 instead of float
# (see also https://github.com/data-apis/array-api/issues/405)
array_api_tests/test_data_type_functions.py::test_finfo[float32-dtype]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alternatively to this mess, considering the ongoing controversy in data-apis/array-api#405, we could relax array-api-tests to check that the output is either a float or a 0-dimensional real floating array.

Copy link
Member

@lucascolley lucascolley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these changes LGTM, but the CI failure looks related

@crusaderky
Copy link
Contributor Author

these changes LGTM, but the CI failure looks related

Yes, the CI failure needs data-apis/array-api-tests#358
Sorry, this one's a mess to merge:

So possibly the best plan is to

  1. wait for next array-api-strict release
  2. merge array-api-tests PR
  3. merge this PR immediately afterwards

@crusaderky
Copy link
Contributor Author

I've updated both this PR and the test in array-api-strict to make merging easier. CI should be green now (but the new feature this PR introduces won't be tested until the array-api-tests PR gets merged).

@ev-br
Copy link
Member

ev-br commented Apr 8, 2025

Okay, let's untie this tangle. Changes LGTM, let's land them now.

@ev-br ev-br merged commit ba0401e into data-apis:main Apr 8, 2025
40 checks passed
@ev-br ev-br added this to the 1.12 milestone Apr 8, 2025
@crusaderky crusaderky deleted the iinfo branch April 8, 2025 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants